Review: PR #73 — languages/rust/stable-structures.md

Must fix

• Upstream comment references dfinity/cdk-rs incorrectly: The page's <!-- Upstream: --> comment lists dfinity/cdk-rs ic-cdk/README.md as a source, but ic-stable-structures is NOT part of the cdk-rs repo. The cdk-rs submodule contains ic-cdk, ic-cdk-timers, and ic-cdk-macros — not ic-stable-structures. The correct upstream source is dfinity/ic-stable-structures. Since that repo is not a submodule, the comment should instead reference the icskills and examples actually used: <!-- Upstream: informed by dfinity/icskills skills/stable-memory/SKILL.md; dfinity/examples rust/daily_planner, rust/unit_testable_rust_canister, rust/tokenmania -->.

• Inline code snippets exceed the 30-line limit: The "Complete canister example" (lines 129–236) is ~109 lines inline. The "Multiple data structures" snippet (lines 248–291) is ~45 lines inline. Project rules state code examples over 30 lines should link to dfinity/examples rather than be inlined. No dedicated stable-structures example exists in .sources/examples/rust/ with #region markers, but the closest analogues (daily_planner, unit_testable_rust_canister) should be linked instead, or the examples should be trimmed and split. At minimum, note in the PR description: "used inline code — no #region markers for a stable-structures example in .sources/examples yet" and comment on issue feat: source code snippets from examples repo with ICP Ninja integration #44 with the needed example.

Suggestions

• Description missing StableLog and StableMinHeap: The frontmatter description reads "Use StableBTreeMap, StableVec, StableCell, and MemoryManager…" but the page also covers StableLog and StableMinHeap. Consider either expanding the description or using a more general phrasing like "Use stable data structures including StableBTreeMap, StableCell, StableLog, and StableVec for upgrade-safe persistent storage in Rust canisters".

• docs.rs URLs for individual types may not resolve: The table in "Available structures" links to deep paths such as btreemap/struct.BTreeMap.html, cell/struct.Cell.html, log/struct.Log.html, vec/struct.Vec.html, and min_heap/struct.MinHeap.html. These module-level URLs are based on the internal module structure of ic-stable-structures and can change between crate versions. They cannot be verified without network access. If any break, they will produce 404s. Consider linking only to the top-level crate root (https://docs.rs/ic-stable-structures/latest/ic_stable_structures/) with a note to search for the type, or adding a <!-- TODO: verify output --> comment near the table for a human to spot-check.

• into_bytes may cause confusion for readers on ic-stable-structures 0.6.x: The Storable implementation includes fn into_bytes(self) -> Vec<u8> which was added in ic-stable-structures 0.7. The examples in .sources/examples/ use 0.6.5 and only implement to_bytes and from_bytes. The page pins to 0.7 in Cargo.toml which is correct, but a brief note that into_bytes is a 0.7 addition would help readers who are upgrading from 0.6.

• "when to use heap vs stable memory" table has a mismatch: The table row "Caches that can be rebuilt after an upgrade" suggests using "transient patterns or heap with recomputation in #[post_upgrade]". The transient keyword is Motoko-specific; it is not a Rust concept. For a Rust page, replace with: "Heap (Vec, HashMap) reconstructed in #[post_upgrade]" to avoid confusion.

Verified

• All icp CLI commands verified against .sources/icp-cli/docs/reference/cli.md: icp network start -d (confirmed -d = --background), icp deploy backend, icp canister call with correct argument syntax — all valid.
• No dfx references. No mo:base imports. No internetcomputer.org/docs/ or docs.internetcomputer.org links.
• Frontmatter is complete: title, description, and sidebar.order are all present.
• <!-- Upstream: --> comment is present at the bottom of the page (CI requirement met), though the cdk-rs reference is incorrect (flagged in Must fix).
• ## Next steps section is present with correct heading variant.
• All internal links verified:
  • ../../guides/backends/data-persistence.md → data-persistence.mdx exists ✓
  • ../../concepts/orthogonal-persistence.md → orthogonal-persistence.md exists ✓
  • ../../guides/canister-management/lifecycle.md → lifecycle.mdx exists ✓ (Astro resolves .md links to .mdx)
• Core API patterns verified against .sources/examples/rust/ (daily_planner, unit_testable_rust_canister, tokenmania):
  • thread_local! { RefCell<MemoryManager<DefaultMemoryImpl>> } pattern ✓
  • StableBTreeMap::init(memory) without .expect() ✓
  • StableCell::init(memory, default).expect(...) — init returns Result, .expect() is correct ✓
  • cell.set(value).expect(...) — set returns Result ✓
  • BOUND: Bound = Bound::Unbounded preferred over Bound::Bounded ✓
  • to_bytes, into_bytes, from_bytes consistent with ic-stable-structures 0.7 Storable trait ✓
  • StableLog two-memory-region pattern (LOG_INDEX_MEM_ID, LOG_DATA_MEM_ID) ✓
• StableVec confirmed to exist in the crate (used in tokenmania example) ✓
• No stub <!-- Content Brief --> or <!-- Source Material --> comments remaining — page is fully written ✓
• No TODO: verify output or Needs human verification flags ✓
• Snippet count: 2 snippets exceed 30 lines (flagged in Must fix); all shorter snippets are within the limit
• Page structure follows orient → explain → instruct → verify → next steps funnel ✓
• "Common mistakes" section proactively covers the hard parts (MemoryId reuse, heap vs stable confusion, Bounded pitfall, missing post_upgrade) ✓

Review: Stable Structures

Must fix

• Frontmatter description promises StableVec coverage that isn't delivered: The description reads "Use StableBTreeMap, StableVec, StableCell, and MemoryManager…" but the page provides no StableVec example. StableVec appears only in the reference table. Either add a brief StableVec usage example or remove "StableVec" from the description. (Content brief also lists StableVec as a required topic.)

• Content brief gap — migration patterns not covered: The stub's content brief explicitly requires "migration patterns between stable structure versions". The page doesn't address this at all: what happens when you add a field to a Storable type between upgrades, why Bound::Unbounded protects against schema evolution failures, or what to do if you need to change a key type. This is a real developer pain point (data can be silently corrupted or lost) and is missing from both the "Common mistakes" section and the main body.

Suggestions

• BTreeSet omission: The portal source and ic-stable-structures crate both list BTreeSet (set operations, deduplication). It's not in the available structures table. This may be an intentional scope decision — but if the page aspires to be the canonical reference for stable structures in Rust, a one-line mention and link would prevent developers from missing it.

• "transient" keyword not explained for Rust context: The heap-vs-stable table mentions transient patterns but transient is a Motoko keyword with no direct equivalent in Rust. A Rust developer reading this may be confused. Rephrase to something like "Heap variables with reinitialization in #[post_upgrade]" to keep it language-appropriate.

• #[post_upgrade] behavioral note could be more precise: The page says omitting #[post_upgrade] "can cause unexpected behavior." This is vague. For stable structures, the data IS still restored without a #[post_upgrade] hook (structures read from stable memory on first access). The real reason to define it explicitly is to re-initialize timers and other transient state. The current phrasing may cause developers to worry unnecessarily or cargo-cult an empty hook for the wrong reason. Suggest: "Omitting #[post_upgrade] is safe for stable data, but timers and other transient state will not be restored — define the hook to handle that reinitialization."

Verified

• All CLI commands verified against .sources/icp-cli/docs/reference/cli.md: icp network start -d, icp deploy backend, icp canister call backend <method> — all correct syntax.
• All three internal links resolve: ../../guides/backends/data-persistence.md → data-persistence.mdx (exists), ../../concepts/orthogonal-persistence.md → orthogonal-persistence.md (exists), ../../guides/canister-management/lifecycle.md → lifecycle.mdx (exists).
• All docs.rs external links use the correct URL pattern per content-authoring.md linking rules table. The ic-stable-structures crate root URL matches the expected pattern exactly.
• No dfx references. No docs.internetcomputer.org links. No mo:base imports. File is .md (not .mdx) — correct.
• Frontmatter has required title and description fields; sidebar.order: 2 is appropriate.
• <\!-- Upstream: --> comment present at end of file (CI-enforced requirement satisfied).
• Rust code patterns verified against .sources/icskills/skills/stable-memory/SKILL.md: thread_local\! { RefCell<StableBTreeMap<...>> } pattern, MemoryManager::init, MemoryId::new(N), StableCell::init(...).expect(...), StableLog::init(...).expect(...) — all match canonical skill patterns.
• Storable trait implementation includes BOUND, to_bytes, into_bytes, and from_bytes — consistent with ic-stable-structures 0.7 trait definition.
• StableCell::set() used with .expect() — correct, as set() returns Result.
• ic_cdk::api::time() — correct API call for timestamp in nanoseconds.
• No pre_upgrade serialization pattern used — correct for stable structures approach.
• ic_cdk::export_candid\!() macro included in the complete example — correct.
• StableLog requires two memory regions demonstrated with two distinct MemoryId constants — correct.
• CBOR serialization via ciborium recommended over Candid for custom types — consistent with skill guidance.
• Bound::Unbounded recommended over Bound::Bounded — correct recommendation with accurate explanation of schema evolution risk.
• Page structure follows orient → instruct → next steps funnel correctly.
• Opening paragraph delivers on the title's promise immediately — no buried lede.

<!-- feedback-addressed -->
Feedback addressed for PR #73 — docs/languages/rust/stable-structures.md

Changes applied

Must fix (Review 1 — marc0olo)

• Removed incorrect dfinity/cdk-rs reference from Upstream comment. The ic-stable-structures crate is not part of cdk-rs. Corrected to reference only dfinity/icskills, dfinity/portal, and dfinity/examples.

• Trimmed inline code examples to address 30-line rule. The "Complete canister example" (~109 lines) is now a focused "Canister wiring example" snippet (~35 lines) that builds on the separately-shown Storable impl, with a link to the full runnable unit_testable_rust_canister example in dfinity/examples. The "Multiple data structures" example (~45 lines) is now a minimal constants-only snippet (~10 lines) with a prose explanation of the StableLog two-region pattern. Note: no dedicated stable-structures example exists in .sources/examples/rust/ with #region markers — used inline code with a GitHub link per content-authoring.md fallback guidance.

Must fix (Review 2 — marc0olo)

• Added StableVec usage section with a working snippet showing push and indexed get, fulfilling the frontmatter description's promise and the content brief requirement.

• Added "Schema evolution" section covering: Bound::Unbounded as the safe default for schema growth; adding fields with Option<T>; migrating key types via a new MemoryId; and the Bound::Bounded max_size risk. This addresses the content brief gap on migration patterns.

Suggestions applied (both reviews)

• Fixed "transient" Motoko keyword in heap-vs-stable table. Replaced with Heap (Vec, HashMap) reconstructed in #[post_upgrade] — language-appropriate for a Rust page.

• Improved #[post_upgrade] common-mistake note. Now accurately states that stable data restores automatically without the hook; the real reason to define it is to re-initialize timers and transient state. Timers silently stop firing after an upgrade if the hook is omitted.

• Added into_bytes version note. Inline comment in the Storable impl clarifies that into_bytes was added in ic-stable-structures 0.7 — helpful for readers upgrading from 0.6.x.

• Added StableBTreeSet to available structures table. The previous table omitted this type.

• Updated frontmatter description to include StableLog and remove the mismatch with page coverage.

• Replaced deep docs.rs type links in Available Structures table with a reference to the top-level crate root URL to avoid 404s if module paths change between crate versions.

Items skipped

None. All feedback items were factually correct and have been addressed either directly or in a cleaner equivalent form.